-
-
Notifications
You must be signed in to change notification settings - Fork 274
Skip generating checksum files for SBOM artifacts in Solaris build #4326
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Thank you for creating a pull request! |
|
@karianna The linter is showing warnings for lines in |
|
Still you can replace modified |
|
@judovana Thank you for your patience. I've implemented the recommended The persistent lint errors are from pre existing lines in the sections I modified. To avoid changing unrelated logic, I'd appreciate any suggestions you might have. |
|
Probably best what you can do, is to open new PR, where - without any logic change, you would fix all the linter issues. Once it is merged, you can rebase this PR, and it will automagically pass. I know it is ungrateful, but best to not mix logical changes and refactoring, and still make all bots happy. Sorry for that :( Thanx a lot for PR! All the best, J. I guess the issues paeared, as there were no changes in this file, and in meantime linter got updated. So new issues popped up. AS there is quite a few of them, would be worthy to fix them for the future. |
| else | ||
| metadata_file="${FILE}.json" | ||
| fi | ||
| createMetadataFile "$metadata_file" "${TARGET_ARCH}" "$SCM_REF" metadata/buildSource.txt metadata/version.txt "$sha256" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is going to make $sha256 in the metadata file an empty string I think, which may cause problems for the API which uses the metadata file. We may have to still generate the sha256 checksum, but not leave the .sha256.txt file around. I'll tag in @andrew-m-leonard to look over this since he's a bit more familiar with this than I am :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sxa Thank you for pointing this out! I’m still learning my way around this part of the codebase, so I really appreciate the guidance. That makes sense I wasn’t fully aware of the impact on the metadata flow and the API. I’ll wait for Andrew’s input and adjust the change accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we should ensure the metadata file has the correct sha256 value in it, but do not generate a sbom.sha256.txt file.
Just remove the sbom.sha256.txt after we have the sha256 value I suspect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@andrew-m-leonard and @sxa Updated the script to generate SHA256 for all files while removing leftover .sha256.txt for SBOMs and setting appropriate metadata filenames. please review.
+1 from me to that, although I'd also be ok with merging this without it first being rebased on a second PR if this hasn't made the linter any worse :-) I thought we'd fixed the linter errors in here somehow. Maybe the linter just got more picky ... EDIT: Yeah https://github.com/adoptium/temurin-build/pull/4254/files should have excluded this file. FYI @adamfarley |
sbin/solaris/build-simple.sh
Outdated
| if [[ "$FILE" =~ .*sbom.*\.json ]]; then | ||
| metadata_file=${FILE%.*}-metadata.json | ||
| # Skip checksum generation for SBOM files | ||
| if [[ "${FILE}" == *sbom.json ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't the correct pattern as the filenames don't end in sbom.json. It needs to match a file like this:
OpenJDK8U-sbom_sparcv9_solaris_hotspot_8u482b01-ea.json
(and the equivalent x64 one instead of sparcv9)
* First draft of version numbers conf parsing script Signed-off-by: Adam Farley <[email protected]> * Second draft of the jdk version file parser This version is far more conservative so as to minimise risk of disruption to the builds. Signed-off-by: Adam Farley <[email protected]> * Linter fixes and improvements Signed-off-by: Adam Farley <[email protected]> * Checking for unbound SCM_REF Signed-off-by: Adam Farley <[email protected]> * Redelimiting unbound variable Signed-off-by: Adam Farley <[email protected]> * Making a zero-length check shorter Signed-off-by: Adam Farley <[email protected]> * Amending null check to include indexing errors Signed-off-by: Adam Farley <[email protected]> * Changing bash delimiters to disable index expansion Signed-off-by: Adam Farley <[email protected]> * Trialling removal of a flaky if condition Signed-off-by: Adam Farley <[email protected]> * Adding quotes for string Signed-off-by: Adam Farley <[email protected]> * Fix for version string parser in build.sh Signed-off-by: Adam Farley <[email protected]> * Refactoring unbound variable handling Signed-off-by: Adam Farley <[email protected]> * Removing superfluous dollar sign Signed-off-by: Adam Farley <[email protected]> * Switching to -v for cross-compatibility Signed-off-by: Adam Farley <[email protected]> * Switching to -z again Signed-off-by: Adam Farley <[email protected]> * Switching to -v Signed-off-by: Adam Farley <[email protected]> * Another try Signed-off-by: Adam Farley <[email protected]> * Trying a for loop Signed-off-by: Adam Farley <[email protected]> * Adding formatting whitespace Signed-off-by: Adam Farley <[email protected]> * Changing default build number to 1 in version string Signed-off-by: Adam Farley <[email protected]> * Handling an incorrect number of arguments Signed-off-by: Adam Farley <[email protected]> * Updating arg validation logic and comments Signed-off-by: Adam Farley <[email protected]> * Switching build_src to camel case Signed-off-by: Adam Farley <[email protected]> * Updated comments Signed-off-by: Adam Farley <[email protected]> * Removing duplicate error reporting logic Signed-off-by: Adam Farley <[email protected]> * Extra comments and restructuring for readability Signed-off-by: Adam Farley <[email protected]> * Extra comments for clarity Signed-off-by: Adam Farley <[email protected]> * Useful comment Signed-off-by: Adam Farley <[email protected]> * Extracting file parsing logic from compareToOpenJDKFileVersion Signed-off-by: Adam Farley <[email protected]> * Removing unused variable Signed-off-by: Adam Farley <[email protected]> * Switching to -n to avoid scm_ref unbound variable issue Signed-off-by: Adam Farley <[email protected]> * Switching version file parser gate to TAG for better locality Signed-off-by: Adam Farley <[email protected]> * Updating jdk version comparison logic for brevity and clarity Signed-off-by: Adam Farley <[email protected]> * Changing if to elif to prevent multiple matches Signed-off-by: Adam Farley <[email protected]> * Quoting grep output to prevent splitting Signed-off-by: Adam Farley <[email protected]> * Adding warning message for jdk version override Signed-off-by: Adam Farley <[email protected]> * Removing excess whitespace Signed-off-by: Adam Farley <[email protected]> * Updating SBOM validator to better handle false negatives Signed-off-by: Adam Farley <[email protected]> * Restructuring SHA check in SBOM validation Signed-off-by: Adam Farley <[email protected]> * Improving posix compliance of SBOM validation Signed-off-by: Adam Farley <[email protected]> * Changing script to use empty strings instead of nulls Signed-off-by: Adam Farley <[email protected]> * Amending curl command for efficiency Signed-off-by: Adam Farley <[email protected]> * Allow for empty scm parameter Signed-off-by: Adam Farley <[email protected]> * Removing unnecessary code Signed-off-by: Adam Farley <[email protected]> * Make code shorter and easier to read Signed-off-by: Adam Farley <[email protected]> * Making code more concise and easier to maintain Signed-off-by: Adam Farley <[email protected]> --------- Signed-off-by: Adam Farley <[email protected]>
…4331) Signed-off-by: Adam Farley <[email protected]>
* All build-configs should use default (bundeld) free type Since jdk8u482.b02 all maintained JDKs have in tree freetype. All jdks should use it unless there is necessary reason to do so. See adoptium#4287 (comment) * Aligned freetypeOnlyBundledOnCertainPlatforms to current state
* Initial minimalistic adaptation of 8 to build with in-tree freetype * Determining free type absed on jdk's 8 tag the if is to long, will refactor * Simplified the condition * The in-tree freetype will appear in 482-b02 * Update sbin/build.sh Co-authored-by: Martijn Verburg <[email protected]> * No longer building freetype if it exists in sources * Get rid of relative path * Aligned modified block to two spaces * Enforced the freetype switches checks * Update sbin/prepareWorkspace.sh Co-authored-by: Martijn Verburg <[email protected]> * Update sbin/prepareWorkspace.sh Co-authored-by: Martijn Verburg <[email protected]> * Update sbin/prepareWorkspace.sh Co-authored-by: Martijn Verburg <[email protected]> * Small rephrasing ow errors * Renamed main freetype methods to match theirs real meaning setDefaultFreeType->setBundledFreeType setFreeTypeFromSrcs->setFreeTypeFromExternalSrcs --------- Co-authored-by: Martijn Verburg <[email protected]>
* Narrowed description of freetype switeches See adoptium#4287 (comment) * On "old-jdk8" the docs man/readme lies - as using the --skip-freetype leads to use system freetype, the --freetype-dir is ignored (which is in ooposite to docs) * When --skip-freetype is omitted, the --freetype-dir works as expected. * On "new-jdk8" and on 11+the usage of -freetype-dir is leading to imminent configure error * with added --skip-freetype its useless as before, as system is used. * Improved grammar of freetype switches description Co-authored-by: Martijn Verburg <[email protected]> --------- Co-authored-by: Martijn Verburg <[email protected]>
* All free type are now bundled, and should be on 2.13.3 * Simplys etting the EXPECTED_FREETYPE=2.13.3 directly with other constants If it goes wild in future, some logic may go back
* cacerts: pull in updated certs from Mozilla * cacerts: pull in updated certs from Mozilla
8aebddb to
bbb2f08
Compare
This PR modifies the Solaris build-simple.sh script to skip generating .sha256.txt checksum files for SBOM JSON artifacts. Previously, the script generated checksums for all artifacts, including SBOMs, which are not intended to be published. Closes #4316
Changes:
Local Testing:
Created a test workspace with dummy artifacts (OpenJDK-fake.tar.gz and OpenJDK-sbom.json).
Verified that:
Note: Reason for opening a new PR
Due to Windows removing the executable flag during local edits, the previous commit did not preserve the executable bit, which caused CI/linter failures. This PR restores the executable permission and includes the SBOM checksum skip changes together.